Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[sonic-config-engine] Clean up dependencies, pin versions; install Python 3 package in Buster container #5656

Merged
merged 24 commits into from
Oct 26, 2020
Merged

Conversation

jleveque
Copy link
Contributor

@jleveque jleveque commented Oct 17, 2020

- Why I did it

To clean up the image build procedure, and let setuptools/pip[3] implicitly install Python dependencies. Also use ipaddress package instead of ipaddr.

- How I did it

- How to verify it

- Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006

@lgtm-com

This comment has been minimized.

@jleveque jleveque changed the title [sonic-config-engine] Clean up dependencies, pin versions [sonic-config-engine] Clean up dependencies, pin versions; install Python 3 package in Buster container Oct 20, 2020
@lguohan
Copy link
Collaborator

lguohan commented Oct 23, 2020

retest this please

@jleveque
Copy link
Contributor Author

Retest broadcom please

@jleveque jleveque marked this pull request as ready for review October 25, 2020 18:03
Copy link
Contributor

@tahmed-dev tahmed-dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

src/sonic-config-engine/minigraph.py Show resolved Hide resolved
src/sonic-config-engine/minigraph.py Show resolved Hide resolved
@jleveque jleveque merged commit 9e34003 into sonic-net:master Oct 26, 2020
@jleveque jleveque deleted the cleanup_cfggen_setup_py branch October 26, 2020 20:48
@vboykox
Copy link
Member

vboykox commented Oct 27, 2020

@jleveque hi, the clean up commit likely causes pmon container to fail because of enum module absence in python2
'docker exec -it pmon python -c "import enum"' fails on the last build and does not fail on previous builds without the commit.

@jleveque
Copy link
Contributor Author

@vboykox: Can you please point to an example of this failure? What you describe was not seen on any of the PR check builds.

@vboykox
Copy link
Member

vboykox commented Oct 27, 2020

@jleveque

SONiC Software Version: SONiC.master.19-dirty-20201027.044155
Distribution: Debian 10.6
Kernel: 4.19.0-9-2-amd64
Build commit: 36c52cca
Build date: Tue Oct 27 13:01:12 UTC 2020
Built by: johnar@jenkins-worker-7

The log:

Oct 27 20:55:25.454183 sonic INFO pmon#supervisord: xcvrd   File "/usr/local/bin/xcvrd", line 24, in <module>
Oct 27 20:55:25.454183 sonic INFO pmon#supervisord: xcvrd     raise ImportError (str(e) + " - required module not found")
Oct 27 20:55:25.454183 sonic INFO pmon#supervisord: xcvrd ImportError: No module named enum - required module not found
Oct 27 20:55:25.543773 sonic INFO pmon#supervisord: xcvrd Traceback (most recent call last):
Oct 27 20:55:25.543773 sonic INFO pmon#supervisord: xcvrd   File "/usr/local/bin/xcvrd", line 24, in <module>
Oct 27 20:55:25.543773 sonic INFO pmon#supervisord: xcvrd     raise ImportError (str(e) + " - required module not found")
Oct 27 20:55:25.543773 sonic INFO pmon#supervisord: xcvrd ImportError: No module named enum - required module not found```

@jleveque
Copy link
Contributor Author

@vboykox: Thank you for reporting this. It looks like this cleanup exposed other areas where dependency specification was lacking. I have created a PR which should fix this: sonic-net/sonic-platform-daemons#106

@vboykox
Copy link
Member

vboykox commented Oct 27, 2020

@jleveque thanks

@jleveque jleveque mentioned this pull request Oct 29, 2020
3 tasks
santhosh-kt pushed a commit to santhosh-kt/sonic-buildimage that referenced this pull request Feb 25, 2021
…thon 3 package in Buster container (sonic-net#5656)

To clean up the image build procedure, and let setuptools/pip[3] implicitly install Python dependencies. Also use ipaddress package instead of ipaddr.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants